-
Notifications
You must be signed in to change notification settings - Fork 1
feat(rust/rbac-registration): RBAC refactoring feature branch #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…in` (#586) * initial * feat: provider * feat: update chain * feat: start_new_chain * feat: merge validation result struct into cip509 * feat: exports * feat: return new chain * chore: return success object * fix: older version * feat: cat id in payload * fix: new version * chore: lintfix * chore: remove persistent arguments * tmp * chore: complete moving to central module * feat: ref fn * chore: merge methods * chore: minor * feat: export modified chains * chore: minor comment * chore: rbac update logic * chore: isolation * chore: validation and lintfix * docs: remove error doc * chore: minor refactor * fix: comments * chore: validation function return * Update rust/rbac-registration/src/providers.rs Co-authored-by: Alex Pozhylenkov <[email protected]> --------- Co-authored-by: Alex Pozhylenkov <[email protected]>
📚 Docs PreviewThe docs for this PR can be previewed at the following URL: |
|
✅ Test Report | |
rust/rbac-registration/src/cardano/cip509/utils/cip134_uri_set.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Rafał Chabowski <[email protected]>
rafal-ch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
| /// Returns a list of all stake addresses. | ||
| /// Returns a set of all active (without taken) stake addresses. | ||
| #[must_use] | ||
| pub fn stake_addresses(&self) -> HashSet<StakeAddress> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
active_stake_addresses would be better
| let previous_addresses = self.stake_addresses(); | ||
| let reg_addresses = cip509.stake_addresses(); | ||
| let new_addresses: Vec<_> = reg_addresses.difference(&previous_addresses).collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding stake will be more readable when glancing through
eg. previous_stake_addresses
| /// Get the latest encryption public key for a role. | ||
| /// Returns the public key and the rotation, `None` if not found. | ||
| #[must_use] | ||
| pub fn get_latest_encryption_pk_for_role( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, if we are using public_key instead of pk this should be renamed, also other pk function too
| ) -> impl Future<Output = anyhow::Result<Option<CatalystId>>> + Send; | ||
|
|
||
| /// Update the chain by "taking" the given `StakeAddress` for the corresponding | ||
| /// RBAC chain's by the given `CatalystId`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is a bit confusing (for me 😅)
| addr: &StakeAddress, | ||
| ) -> impl Future<Output = anyhow::Result<bool>> + Send; | ||
|
|
||
| /// Returns a corresponding to the RBAC chain's Catalyst ID corresponding by the given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one too. Maybe
"Returns the Catalyst ID associated with the RBAC chain for the given signing public key"
Description
Related Pull Requests
RegistrationChain#586StakeAddresshandling. #631Cip0134UriSet#655Cip0134UriSetdata #658Please confirm the following checks